[compiler] Extend setState in effect validation to useEffectEvent #35214
Conversation
ValidateNoSetStateInEffects already supports transitive setter functions. This PR marks any synchonous state setter useEffectEvent function so we can validate that uEE isn't being used only as misdirection to avoid the validation within an effect body. The error points to the call of the effect event.
| ? instr.value.receiver | ||
| : instr.value.callee; | ||
|
|
||
| if (isUseEffectEventType(callee.identifier)) { |
There was a problem hiding this comment.
This would only catch useEffectEvent function type calls at the top level in a useEffect right? Should we consider something like this?
const [state, setState] = useState(0)
const effectEvent = useEffectEvent(() => {
setState(true)
});
useEffect(() => {
function foo() {
effectEvent()
}
someLogic();
foo();
}, [])
I'm just curious. Would this code even work? I'm not very familiar with uEE
There was a problem hiding this comment.
I don't think there's anything to specific about uEE in that example: it's just a question of whether we check recursive function expression (looks like maybe not, would be an easy extension). But that's an orthogonal change.
There was a problem hiding this comment.
I double-checked, we don't currently catch this case if you swap the effectEvent() call for a direct setState() call. Definitely worth adding — great catch, @jorge-cab! — but not strictly related to this PR
| setStateFunctions.set(instr.lvalue.identifier.id, setState); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
can do else if to connect w the existing condition, since they can't both be true
|
I actually bumped into this one today and I'm not sure what the intended solution for "did mount" flag is. Specifically, if you need to use some browser-only stuff in JSX, so you're forced to separate hydration in two stages (initial server content -> client content). See https://react.dev/reference/react/useEffect#displaying-different-content-on-the-server-and-the-client for this use case described in the docs. |
4 participants
ValidateNoSetStateInEffects already supports transitive setter functions. This PR marks any synchonous state setter useEffectEvent function so we can validate that uEE isn't being used only as misdirection to avoid the validation within an effect body.
The error points to the call of the effect event.
Example: